[HIGH] Idempotency guard has no author/commit filter and fails open on its own snapshot query

Finding: The new idempotency guard (_last_review_id snapshot at lines 343-345, comparison at lines 379-388) treats any increase in max(.id) across all PR reviews as proof that this run's own fullsend post-review call succeeded server-side — it never filters by posting identity or by commit_id matching the reviewed SHA. Two independent ways this produces a false "already posted" conclusion, both of which set POST_REVIEW_EXIT=0 and fall straight into the unconditional "Outcome labels" section: (1) if a human or another bot submits any review on the PR during the 5-20s backoff window, the guard concludes the failed attempt "actually succeeded"; (2) the initial snapshot swallows gh api failures via 2>/dev/null || true, silently defaulting _last_review_id to 0 — on a PR that already has prior reviews (e.g. from an earlier /fs-review redispatch cycle, which this pipeline explicitly supports), the very next per-retry check will see _current_review_id > 0 and falsely conclude success on the first failed attempt. In both cases the only trace is a low-visibility ::notice::, not a ::warning::/::error:: — the intended review is never posted, yet the workflow reports success and can still attach an outcome label.

Suggestion: Filter the reviews query to this run's own identity (cache gh api user --jq .login once, or resolve the bot login) and/or match on commit_id equal to the reviewed head SHA, e.g. --jq --arg sha "$HEAD_SHA" 'map(select(.commit_id == $sha)) | map(.id) | max // 0'. Treat a failed snapshot query as "unknown" rather than 0, since 0 makes the guard trivially satisfied by any pre-existing review.

Independently flagged by all 4 reviewers in this pass (Claude x2, Grok, Codex); one reproduced the false-positive empirically by simulating a permanent fullsend failure alongside an incrementing (but unrelated) reviews-API mock.

[HIGH] Degraded-mode fallback applies the outcome label without confirming the review comment was actually posted

Finding: Issue #159's proposed change and validation criteria both explicitly condition the fallback on the comment having been posted ("if all retries are exhausted but the review comment was already posted successfully, attempt to apply the label"). The implementation drops that condition: the comment block above this line says outright "We cannot distinguish 'comment posted, review failed' from 'nothing posted'... Applying the label here is a best-effort measure" — and then applies it unconditionally whenever all 3 attempts exit non-zero/non-10. Tracing fullsend post-review's actual implementation confirms this matters: it posts the sticky comment first and returns immediately if that call itself fails, before ever attempting the formal review. So a failure mode common to all 3 attempts (e.g. an invalid/expired token, or the API being entirely unreachable) means nothing is posted to the PR — no comment, no review — yet the fallback still applies e.g. ready-for-merge for an approve verdict, purely from the sandboxed agent's proposed JSON action. The only signal anything went wrong is the workflow's exit code, which is easy to miss if downstream automation gates on label presence — the exact failure mode issue #159 was originally filed over.

Suggestion: Before applying any fallback label, verify a sticky comment actually landed — e.g. check the PR's issue comments for the <!-- fullsend:review-agent --> marker the CLI itself uses — and only proceed with the fallback if the marker is present, matching the issue's stated condition.

Flagged by 1 of 4 reviewers, who traced the actual call order and marker constant in the fullsend CLI source; independently confirmed here against the same source and against issue #159's text.

[MEDIUM] Reviews-list API calls omit --paginate, silently capped at the first 30 (oldest) reviews

Finding: Both new gh api repos/.../pulls/.../reviews calls (lines 343, 381) fetch only GitHub's default first page — 30 items, oldest-first — unlike the other multi-page listing calls already in this file (labels --paginate, issues/.../comments --paginate). Once a PR accumulates more than 30 reviews over its lifetime (plausible for a heavily re-reviewed/re-dispatched PR under this same pipeline), max(.id) stays pinned to the same stale page-1 value on every check, and the -gt comparison can never fire — silently disabling the idempotency guard on exactly the busy, long-lived PRs most likely to need it.

Suggestion: Add --paginate to both calls, consistent with the existing convention in this file.

Flagged by 3 of 4 reviewers; one reproduced it directly (35 simulated reviews -> unpaginated query returns 30 regardless of the true max).

[MEDIUM] Idempotency check is never re-run before the final "retries exhausted" branch

Finding: The idempotency re-check only runs inside if [ "${_pr_attempt}" -lt "${POST_REVIEW_MAX_ATTEMPTS}" ] (line 379) — i.e. between attempts 1->2 and 2->3. On the last attempt, a failure falls straight into this else branch ("all retries exhausted") with no re-query, then into the degraded-mode fallback and exit 1. If attempt 3 itself is a false negative — the server processed the write but the CLI reported failure, the exact scenario this whole PR exists to handle — the script has no way to detect it at the one moment it matters most, and reports a failure (re-applying a label via the fallback) for a run that may have actually succeeded.

Suggestion: Run the same review-ID comparison once more immediately after the final attempt fails, before falling into the elif [ "${POST_REVIEW_EXIT}" -ne 0 ] branch, so a genuinely successful last attempt is still reported as success.

Flagged by 3 of 4 reviewers.

[MEDIUM] premature-decision — No test exercises the idempotency guard's actual trigger path

Finding: This mock's reviews-API handler (and its non-retry counterpart earlier in the file) unconditionally echo "0"s regardless of call count. Across all 7 retry tests plus the 3 backoff/validation tests, _current_review_id can therefore never exceed _last_review_id — the actual anti-double-post branch this PR adds in post-review.sh (POST_REVIEW_EXIT=0; break on the idempotency-guard hit) has zero test coverage. This is the same untested-safety-mechanism pattern that let the original crash bug ship unnoticed last round: the guard added specifically to close a HIGH-severity risk is itself shipped with no test proving it fires.

Suggestion: Add a mock reviews handler that returns an incrementing max-id on a later invocation (mirroring the existing MOCK_FULLSEND_COUNTER pattern), and a test asserting the "idempotency guard" notice appears and no further fullsend call happens after it fires.

Flagged by all 4 reviewers.

[MEDIUM] Raw fullsend stderr is interpolated into GHA ::warning:: annotations without the sanitization used elsewhere in this file

Finding: _pr_err_detail is built from unsanitized fullsend stderr and embedded directly into the retry ::warning:: messages a few lines below. Elsewhere in this same file, external/derived strings destined for GHA workflow-command output are explicitly stripped of \n, \r, and :: before use — see LA_ACTION/LA_LABEL earlier in the file, and _safe_action (added by this very PR for a sibling message in the same fallback block, just further down). Since GitHub's validation-error bodies can echo back parts of the request that triggered them, a 422 whose message reflects PR-controlled input could plausibly propagate an unsanitized :: sequence into a workflow-command annotation.

Suggestion: Apply the same \n/\r/::-stripping already used for _safe_action to _pr_err_detail before interpolating it into the ::warning:: messages.

Flagged by 1 of 4 reviewers; verified here against this file's own established sanitization pattern.

[MEDIUM] premature-decision — the "~20s worst case" comment doesn't account for fullsend's own internal HTTP retry/backoff

Finding: This comment states a permanent failure "wastes at most two retry attempts (~20s)." That holds only for failure classes the fullsend CLI doesn't already retry internally. fullsend's own GitHub client retries 429/5xx/secondary-rate-limited-403 responses internally (maxRetries = 3 in its GitHub client — not 5, as initially reported to me and corrected after checking the source directly), and for secondary rate limits specifically uses a 60s-plus-jitter base backoff — so a single fullsend post-review invocation can itself take roughly two minutes to give up before ever returning control to this script. Stacking this shell-level retry loop (3 attempts) on top means the true worst case under sustained rate-limiting is on the order of several minutes, not "~20s" — though 422 (this PR's specific motivating failure) is not retried internally by the Go client, so the common case is unaffected.

Suggestion: Scope the "~20s" claim to failure classes the CLI doesn't already retry (e.g. 422, auth errors), or drop the specific number and note that total worst-case latency depends on fullsend's own internal retry behavior for rate-limit-class failures.

Flagged by 1 of 4 reviewers, who traced the retry constants in the fullsend CLI source; re-verified directly against that source here.